Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coordinate system alignment to specific markers or between scenes #695

Merged
merged 11 commits into from
Sep 26, 2019

Conversation

fabiencastan
Copy link
Member

@fabiencastan fabiencastan commented Sep 22, 2019

Description

Use CCTag markers to define the coordinate system: the SfMTransform node provides new inputs to align specific markers to known 3D positions.

Features list

  • StructureFromMotion node now computes ID for CCTags and store it in the landmark color (markers have no interesting color, so this field is re-used for the ID).
  • SfMTransform provides a list of markers/3D positions to redefine the coordinate system of the reconstruction.
  • SfMAlignment provides an option to align based on markers ID
  • SfMAlignment provides an option to align based on poseId
  • SfMAlignment provides an option to align views based on file pattern matching (regex)
  • SfMAlignment provides an option to align views based on metadata maching
  • New SfMTransfer software to retrieve poses and intrinsics from another reconstruction based on matching views

Implementation remarks

Markers IDs are stored in the landmark color to avoid the creation of an additional field which would take space for nothing on all landmarks. Maybe in the future, we could consider to have multiple types of landmarks (with different data structure) but that would represent a complex change for a limited benefit for now.

  • minor warning fix in cctag
  • minor check added in Texturing
  • SfM: bug fix tracksPerView should have a key for each view
  • SfM fix: this is not an error if we have a calibration in input

…ent: views ids, poses ids, filepath pattern, specific metadata, markers ids

- SfMAlignment: provides an option to align based on markers ID
- SfMAlignment: provides an option to align based on poseId
- SfMAlignment: provides an option to align views based on file pattern matching (regex)
- SfMAlignment: provides an option to align views based on specific metadata matching
if (d[i] == 255)
{
ALICEVISION_LOG_TRACE("Found marker: " << i << " (landmarkId: " << landmarkIt.first << ").");
landmark.rgb.r() = i;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we just add another field to landmark like markerId always initialized to a dummy value (for all the other features that are not markers)?
Using the color attribute is a kind of workaround. Also, it is limited to 255 values, which is limiting in case other types of markers will be integrated in the future (eg. aruco &C).
Ok we need to change all the IO functions of sfmdata but I think it's better than "hiding" data in the same structure. And I think we can still preserve backcompatibility with other sfmdata files that has not that field.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I explained in the PR summary, it will be a waste of space as most of the landmarks are not markers. I agree that it looks hackish. I was considering using an union between RGBColor and int but that represents many changes in IOs and creates complexity.
The correct long-term solution is probably to have landmarks per descType, but that's a major change that I cannot afford for now.

… from another reconstruction based on matching views
In many places, we assume that tracksPerView has a key for each view.
So this commit adds an initialization step to ensure that it is the case
(instead of checking if the key exists all over the place).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants